Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(graphcache) - fix no selectionset mutation updates in optimistic phase #654

Merged
merged 4 commits into from
Mar 22, 2020

Conversation

JoviDeCroock
Copy link
Collaborator

Summary

Their was a scenario where a mutation without selectionset would cause a call to its respective updater function.

mutation {
  someUpdate
}

As you can see the above has no selectionset, this would cause the writeSelectionSet not to check if there was an actual optimistic-entry for the current rootField

Fixes: #653

Set of changes

  • fix no selectionset mutation updates in optimistic phase

@JoviDeCroock JoviDeCroock requested a review from kitten March 21, 2020 21:29
@changeset-bot
Copy link

changeset-bot bot commented Mar 21, 2020

🦋 Changeset is good to go

Latest commit: 0d05c91

We got this.

This PR includes changesets to release 1 package
Name Type
@urql/exchange-graphcache Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@JoviDeCroock
Copy link
Collaborator Author

I fixed the codesandbox in this PR #656

@@ -247,7 +247,7 @@ const writeSelection = (
} else if (entityKey && !isRoot) {
// This is a leaf node, so we're setting the field's value directly
InMemoryData.writeRecord(entityKey || typename, fieldKey, fieldValue);
}
} else if (ctx.optimistic && isRoot) continue;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we leave a TODO here for scalar optimistic results or just address that in a subsequent PR?
I’d have time to take a look at that, because I suppose it’s expected behaviour 🤷‍♀️

Copy link
Collaborator Author

@JoviDeCroock JoviDeCroock Mar 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to defer this to a later PR, if you don't get to it I can also look into it no pressure 😄

@JoviDeCroock JoviDeCroock merged commit 56193e4 into master Mar 22, 2020
@JoviDeCroock JoviDeCroock deleted the fix/no-selectionset-updates branch March 22, 2020 00:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[graphcache] mutation update executed twice
2 participants